Feature/mnaveau/doc#307
Conversation
…agimus-controller-doc
|
@Mergifyio rebase |
☑️ Nothing to do, the required conditions are not metDetails
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Sphinx documentation package for agimus_controller, wires it into the Nix flake as a buildable derivation, and adds extensive in-code docstrings to the OCP modules to support API generation.
Changes:
- Add a new
agimus_controller_doc/documentation package (Sphinx config, markdown sources, autosummary/autodoc pages, CLI helper). - Add a Nix derivation and flake output to build and publish HTML docs.
- Add/expand docstrings in
ocp_croco_generic.pyandocp_croco_generic_force_feedback.pyto improve generated API documentation; update ROS test dependencies.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| flake.nix | Exposes agimus-controller-doc as a flake package output. |
| agimus_controller_doc/pyproject.toml | Defines Poetry package for docs build tooling and dependencies. |
| agimus_controller_doc/docs/requirements.txt | Provides pip-style requirements for building docs. |
| agimus_controller_doc/docs/index.md | Adds Sphinx docs landing page with toctree. |
| agimus_controller_doc/docs/general/usage.md | Adds a brief usage page and YAML snippet example. |
| agimus_controller_doc/docs/conf.py | Adds Sphinx configuration (extensions, sys.path handling). |
| agimus_controller_doc/docs/api/ocp_croco_generic.md | Adds a module overview page and automodule API extraction. |
| agimus_controller_doc/docs/api/index.md | Adds API index toctree for autosummary/autoclass pages. |
| agimus_controller_doc/docs/api/ResidualModelState.md | Autoclass stub page for ResidualModelState. |
| agimus_controller_doc/docs/api/ResidualModelControl.md | Autoclass stub page for ResidualModelControl. |
| agimus_controller_doc/docs/api/ResidualModelControlGrav.md | Autoclass stub page for ResidualModelControlGrav. |
| agimus_controller_doc/docs/api/ResidualModelFramePlacement.md | Autoclass stub page for ResidualModelFramePlacement. |
| agimus_controller_doc/docs/api/ResidualModelFrameTranslation.md | Autoclass stub page for ResidualModelFrameTranslation. |
| agimus_controller_doc/docs/api/ResidualModelFrameRotation.md | Autoclass stub page for ResidualModelFrameRotation. |
| agimus_controller_doc/docs/api/ResidualModelFrameVelocity.md | Autoclass stub page for ResidualModelFrameVelocity. |
| agimus_controller_doc/docs/api/ResidualDistanceCollision.md | Autoclass stub page for ResidualDistanceCollision. |
| agimus_controller_doc/docs/api/ResidualDistanceCollision2.md | Autoclass stub page for ResidualDistanceCollision2. |
| agimus_controller_doc/docs/api/ActivationModelWeightedQuad.md | Autoclass stub page for ActivationModelWeightedQuad. |
| agimus_controller_doc/docs/api/ActivationModelExp.md | Autoclass stub page for ActivationModelExp. |
| agimus_controller_doc/docs/api/ActivationModelQuadExp.md | Autoclass stub page for ActivationModelQuadExp. |
| agimus_controller_doc/docs/api/CostModelResidual.md | Autoclass stub page for CostModelResidual. |
| agimus_controller_doc/docs/api/CostModelSumItem.md | Autoclass stub page for CostModelSumItem. |
| agimus_controller_doc/docs/_static/.gitkeep | Ensures _static/ is tracked in git. |
| agimus_controller_doc/default.nix | Adds a Nix derivation that builds Sphinx HTML docs. |
| agimus_controller_doc/agimus_controller_doc/cli.py | Adds a CLI entrypoint to build docs via Sphinx. |
| agimus_controller_doc/agimus_controller_doc/init.py | Initializes the docs helper Python package. |
| agimus_controller_doc/README.md | Documents ways to build/host the docs (venv, Nix, Poetry). |
| agimus_controller/package.xml | Adds tiago_robot as a test dependency. |
| agimus_controller/agimus_controller/ocp/ocp_croco_generic_force_feedback.py | Adds module/class documentation for force-feedback OCP components. |
| agimus_controller/agimus_controller/ocp/ocp_croco_generic.py | Adds module/class documentation for generic OCP dataclass builders and models. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class ActivationModelWeightedQuad(ActivationModel): | ||
| class_: T.ClassVar[str] = "ActivationModelWeightedQuad" | ||
| weights: T.Union[None, float, npt.NDArray[np.float64]] = None | ||
| """Weighted quadratic activation. | ||
|
|
There was a problem hiding this comment.
The triple-quoted string here is not a class docstring because it appears after assignments (class_ / weights). It becomes a no-op string literal at runtime and won’t show up in generated docs. Move this text to be the first statement in the class body (immediately after the class ...: line) or convert it into a real comment/docstring structure.
| class ActivationModelExp(ActivationModel): | ||
| class_: T.ClassVar[str] = "ActivationModelExp" | ||
| alpha: float = 1.0 | ||
| exponent: int = 1 | ||
| """Exponential-like activation. |
There was a problem hiding this comment.
This triple-quoted block is not a class docstring because it is not the first statement in the class body (it comes after attribute assignments). It becomes an unused string literal and won’t show up in autodoc/help(). Move it to immediately after class ActivationModelExp...:.
| class_: T.ClassVar[str] = "DifferentialActionModelFreeFwdDynamics" | ||
| costs: T.List[CostModelSumItem] | ||
| constraints: T.List[ConstraintListItem] = dataclasses.field(default_factory=list) | ||
|
|
||
| """Differential action model using free forward dynamics. |
There was a problem hiding this comment.
This triple-quoted block is not a docstring (it appears after field annotations), so it will be ignored by autodoc and kept as an unused string literal. Move it to be the first statement in the class body.
| return iam | ||
|
|
||
| """Integrated action model that wraps a soft-contact augmented DAM. | ||
|
|
||
| This wrapper integrates the augmented differential action model using the |
There was a problem hiding this comment.
This triple-quoted block is not a docstring for IAMSoftContactAugmented (it is placed after methods). It will be ignored by autodoc and kept as an unused string literal. Move it to immediately after the class IAMSoftContactAugmented: line.
| 2. Build HTML: | ||
|
|
||
| ```bash | ||
| make html | ||
| # Output is in _build/html | ||
| ``` |
There was a problem hiding this comment.
The README suggests make html, but there is no docs/Makefile in this package, so this command will fail. Either add the standard Sphinx Makefile under docs/ or change the instructions to use sphinx-build -b html docs docs/_build/html (or the Poetry/Nix commands below).
|
|
||
| # Compute and add repository root to sys.path so autodoc can import | ||
| # the `agimus_controller` package regardless of current working dir. | ||
| ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) |
There was a problem hiding this comment.
ROOT is described as the repository root, but ../../ from agimus_controller_doc/docs/conf.py resolves to agimus_controller_doc/, not the actual repo root. If agimus_controller is not installed in the environment, autodoc imports will fail because the sibling agimus_controller/ package won’t be on sys.path. Consider adjusting the path (e.g., one more .. or explicitly adding ../agimus_controller) or updating the comment to reflect the intended build assumptions (Nix/Poetry-installed package).
| ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) | |
| ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "..")) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Kotochleb @ahaffemayer @MaximeSabbah @petrikvladimir : I through the base of the documentation of the different costs. Please review the MAth in case of error. This was AI generated very quickly.
@nim65s I through the base of the nix build of the agimus_controller_doc.
@petrikvladimir please review the PR and tell me if this documentation is enough for you or if you need more details in the API usage.